Enforce progressive-list soft limits at network decode time - #11263
Conversation
| sources: | ||
| - file: ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/util/GloasNetworkLimits.java | ||
| search: public static Optional<LimitViolation> verifyBlockBodyOperationLimits( | ||
| - file: ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/util/GloasNetworkLimits.java |
There was a problem hiding this comment.
1st ref looks sufficient, as it contains most of the logic linked to the spec method
| sources: | ||
| - file: ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/util/GloasNetworkLimits.java | ||
| search: public static Optional<LimitViolation> verifyExecutionRequestsLimits( | ||
| - file: ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/util/GloasNetworkLimits.java |
There was a problem hiding this comment.
1st ref looks sufficient, as it contains most of the logic linked to the spec method
| this(beaconBlockSchema, containerName, OptionalLong.empty(), Optional.empty()); | ||
| } | ||
|
|
||
| public SignedBeaconBlockSchema( |
There was a problem hiding this comment.
Maybe add Javadocs to different constructors, something like validation required starting from Gloas or smth like that
| try { | ||
| return type.sszDeserialize(message); | ||
| final T decoded = type.sszDeserialize(message); | ||
| type.getNetworkSszValidator().ifPresent(validator -> validator.validate(decoded)); |
There was a problem hiding this comment.
if exception is thrown in validator, description is lost in catch
| markAsInvalid(); | ||
| } else if (rootCause instanceof MalformedDataException) { | ||
| LOG.debug("Malformed response received while requesting batch data", error); | ||
| markAsInvalid(); |
There was a problem hiding this comment.
Now any MalformedDataException causes up to 12 hours peer ban which includes for example MessageTruncatedException. I'd carefully review list affected exception and also add JavaDoc to markAsInvalid() so everyone knows which kind of penalty is it.
2 participants
Makes sure that limits that are part of the state transition function are applied at message decoding phase (GOSSIP as well as RPC) so we early drop them. REJECT and peer downscore during sync (RPC) are ensured.
The alternative is just reordering the gossip checks so that those limits are checked earlier.
This is more invasive approach, tightening the check to the schema itself, bringing the benefit that we apply it on RPC side too (we don't need to fill a batch, start the import, to find out that some data is invalid)
related to #11228
ref: ethereum/consensus-specs#5564
Out of scope, DAS column retrieval and other RPC consumers outside forward sync do not penalize malformed responses either; a central hook in
DefaultEth2Peer.sendEth2Requestcould cover them later.Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog
Note
Cursor Bugbot is generating a summary for commit c3eede3. Configure here.